Add E2E tests for appointment booking workflow - #16632

Broken (correctness) — these tests can pass without testing anything.

Every meaningful action in this file is wrapped in if (await X.isVisible()). If the practitioner selector never renders (regression, slow load, changed label), the block is skipped, the step is empty and the test goes green. The same shape repeats in every test.

A test that silently no-ops on the failure it exists to catch is worse than no test: it reports coverage that does not exist. Since the strings are known (select_practitioner / select_healthcare_service in public/locale/en.json), assert unconditionally:

await expect(resourceTrigger).toBeVisible();
await selectFromCommand(page, resourceTrigger, { itemIndex: 0 });

If the concern is that the selector varies by resource type, pick the type explicitly first (select_resource_type in AppointmentFormSection.tsx) so the test is deterministic rather than conditional.

Broken — this step asserts nothing.

[role='button'][name*='day'] is a CSS attribute selector for a literal name attribute; <button> elements do not have one, so count() is always 0, calendarExists is always false, and the if body never runs. Even if it did, expect(calendarExists).toBe(true) inside if (calendarExists) is a tautology.

The date UI lives in AppointmentDateSelection.tsx and is reachable via the select_date label in BookAppointmentDetails.tsx — assert on that unconditionally instead.

Broken — count() does not wait, so this races the slot query.

locator.count() returns immediately with whatever is in the DOM at that instant. Slots are fetched async (slotsQuery in AppointmentSlotPicker.tsx), so on a normal run slotCount is 0, the block is skipped and the test passes without ever selecting a slot — the exact thing it claims to cover. The preceding waitForTimeout(1000) makes this timing-dependent rather than deterministic.

Prefer waiting on a real signal, e.g. await expect(slotButtons.first()).toBeVisible() (or the no_slots_available_for_this_date empty state), then act.

Separately, /create appointment|book|confirm/i is loose: the real label is confirm_appointment, and book also matches the sheet's Book Appointment heading/tab, risking a strict-mode violation. Use the actual name with exact: true.

Broken — this assertion cannot fail in a useful way.

The test is named "should handle no available slots gracefully", but expect(slotCount > 0 || hasEmptyMessage).toBe(true) passes in either world, so it never distinguishes the empty state from the populated one. Combined with the non-waiting count() above, the likely real outcome is that neither branch is genuinely observed.

To actually test the empty state you need to drive the app into it (pick a date with no schedule) and then assert on no_slots_available_for_this_date from AppointmentSlotPicker.tsx directly. As written the test should either be made deterministic or dropped.

Convention — tests/PLAYWRIGHT_GUIDE.md (Common Pitfalls #7) says to avoid hardcoded timeouts and rely on visibility checks or global config timeouts. This file adds four waitForTimeout calls (1000/1000/500/2000) plus explicit { timeout: 2000 } on the sheet-close assertions; the whole existing tests/ tree has only 5 waitForTimeout calls total. These are also the mechanism by which the conditional blocks above end up skipped. Replace each with a wait on the thing you actually need (expect(...).toBeVisible() / waitForLoadState).

Approach — the "open booking sheet" block and the practitioner-selection block are copy-pasted verbatim across 5–6 tests. Extract a small local helper (openBookingSheet(page) returning the sheet locator, and selectResource(sheet)) at the top of the file. That removes ~60 of the 365 lines and means the selector fixes from the comments above only need applying once.

While there: this file duplicates two whole tests. "should select appointment date and time slot" and "should complete full appointment booking workflow" run the same steps, the latter just continuing further. Keep the end-to-end one and drop the prefix.

Convention — getByRole("tab", { name: /bookings/i }) will also match the Book Appointment tab? No — but /book appointment/i on line 254/276 matches both the SheetTitle heading and the tab only because you scope by role, which is fine. The real risk is the missing exact on /bookings/i, per PLAYWRIGHT_GUIDE.md pitfall #1. Since both labels come from en.json (book_appointment, bookings), prefer exact names over case-insensitive regex so a copy change fails loudly instead of silently matching the wrong tab.